CI: https://ci.nodejs.org/job/node-test-pull-request/32312/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/3241/

/cc @gengjiawen related to nodejs/node-v8#161

/cc @gengjiawen related to nodejs/node-v8#161

Yeap. Also msvc only report one error, I will give it a try.

BTW, we need to cherrypick v8/v8@4ece106.

@targos, since --experimental-wasm-bigint is no longer required for the WASI API, would you like to pull cjihrig@dbff5bc into this upgrade, or would you prefer me to PR it separately after the upgrade lands?

In case you want patch code for windows, you can get first two commit in https://github.com/gengjiawen/node/commits/v8-8.5-hack before nodejs/gyp-next#60 get fixed. @targos

Is there any chance that it may get into Node 14 or 15? 😶

Testing something: https://ci.nodejs.org/job/node-test-commit-arm-fanned/16538/

@targos I believe I have find the root cause through the generated mk file. It looks like the assembly file processed both on host and target. So we should limit it only on target (<(V8_ROOT)/src/heap/base/asm/arm/push_registers_asm.cc are valid assembly only for arm, that's why gcc failed on v8_base_without_compiler.host.mk) .

Below is build by removing it in v8_base_without_compiler.host.mk file manully.

Maybe only something like (not quite sure about the syntax)

              ['_toolset == "target" and v8_current_cpu=="arm"', {
                'sources': [
                  '<(V8_ROOT)/src/heap/base/asm/arm/push_registers_asm.cc'
                ],
              }],

I don't know... My condition seems right. I looked at the Jenkins logs and for cross-compiler-ubuntu1804-armv7-gcc-6, host_arch is ia32 and target_arch is arm, so unless I'm missing something with priority of operators in Python, it should pick up the ia32 file.

So, with the log I added, we can see that the variables have the following values:

• 'host_arch': 'ia32'
• 'target_arch': 'arm'
• '_toolset': 'target'

There's no other log where _toolset is host 🤔. I'm trying one last thing.

CI: https://ci.nodejs.org/job/node-test-commit-arm-fanned/16551/

Edit: doesn't work. I'm out of ideas.

@targos if you want to test locally, you can also try https://github.com/mmarchini/node-arm-cross-builder

_toolset being always target seems like either a bug, or we're using the wrong variable to test the current toolset.

_toolset being always target seems like either a bug, or we're using the wrong variable to test the current toolset.

_toolset seems to be 'target' all the time from what I test. Maybe a bug in gyp.

Looks the generated mk file toolset is set correctly. Where _toolset is set in gyp ?

  1 # This file is generated by gyp; do not edit.
  2 
  3 TOOLSET := host
  4 TARGET := v8_base_without_compiler

I realise I'm somewhat "drive-by commenting" here, but is there a reason why we still have to cross compile for arm32? There's some reasonably quick stuff out there now (e.g. Raspberry Pi4 or ODROID-XU4 which I use) and if those could be used to build natively in a reasonable amount of time and don't exhibit the same problems it might be worth switching.

So, with the log I added, we can see that the variables have the following values:

• 'host_arch': 'ia32'
• 'target_arch': 'arm'
• '_toolset': 'target'

There's no other log where _toolset is host 🤔. I'm trying one last thing.

CI: ci.nodejs.org/job/node-test-commit-arm-fanned/16551

Edit: doesn't work. I'm out of ideas.

Tinker v8.gyp like will have '_toolset': 'host' in the log, but I still get v8_base_without_compiler.host.mk have $(obj).host/$(TARGET)/deps/v8/src/heap/base/asm/ia32/push_registers_asm.o, not sure why.

    {
      'target_name': 'v8_cppgc_shared',
      'type': 'none',
      # 'conditions': [
      #   ['want_separate_host_toolset', {
      #     'toolsets': ['host', 'target'],
      #   }],
      # ],
      'toolsets': ['host', 'target']

@sxa It's already tier 1 in build. I suppose even if we want to change it. It will be a much more bigger issue.

Context: nodejs/TSC#931 (comment).

For completeness, link to a proposed switch to building natively that went stale nodejs/build#1851

@targos I think I find the fix, not sure the reason why. I tested in the docker, works fine, you can test it in Jenkins.

Change the part like this

    {
      'target_name': 'v8_cppgc_shared',
      'type': 'none',
      # 'conditions': [
      #   ['want_separate_host_toolset', {
      #     'toolsets': ['host', 'target'],
      #   }],
      # ],
      'toolsets': ['host', 'target'],
      'direct_dependent_settings': {
        'sources': [
          '<(V8_ROOT)/src/heap/base/stack.cc',
          '<(V8_ROOT)/src/heap/base/stack.h',
        ],
        'conditions': [
          ['clang or OS!="win"', {
            'conditions': [
              ['_toolset=="target" and target_arch == "arm"', {
                'sources': [
                  '<(V8_ROOT)/src/heap/base/asm/arm/push_registers_asm.cc',
                ],
              }],
              ['_toolset == "target" and target_arch=="arm64"', {
                'sources': [
                  '<(V8_ROOT)/src/heap/base/asm/arm64/push_registers_asm.cc',
                ],
              }],
            ]
          // other omitted.

@gengjiawen with the change you propose, does it still work for regular compilation, for example x64?

@gengjiawen with the change you propose, does it still work for regular compilation, for example x64?

Not test it yet, give me a second.

@gengjiawen with the change you propose, does it still work for regular compilation, for example x64?

@targos Looks like still works (Note: in my snippet I only show two cpus).

PS: the full build without ccache is really long.

Let's try it!

CI: https://ci.nodejs.org/job/node-test-pull-request/33285/

It works!!! Thank you @gengjiawen ❤️ . Now I'm going to open a new PR to update directly to 8.6, because it will be stable next week and we need it ASAP for v15.

It works!!! Thank you @gengjiawen ❤️ . Now I'm going to open a new PR to update directly to 8.6, because it will be stable next week and we need it ASAP for v15.

You want to land this first or drop it ?

It's easier for me to drop it.

PR for 8.6: #35415